Durga js variables#533
Conversation
Summary of ChangesHello @Saidurgasatturi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a series of new assignments designed to enhance understanding of fundamental web development concepts. It includes practical examples for CSS layouts, animations, and a comprehensive set of JavaScript basics, such as variable handling, data types, type conversion, and modern string manipulation with template literals. The additions aim to provide clear, executable examples for learning these core topics. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds several assignments related to CSS and JavaScript variables and data types. The code demonstrates various concepts, but there are several areas for improvement regarding best practices and correctness.
My review includes the following points:
- Critical Bug: A typo in
innerHTMLprevents a script from running correctly. - Incorrect Logic: The demonstration of
letandconstredeclaration is flawed, as it doesn't correctly show the error that would be thrown. - Best Practices:
- Several files mix JavaScript and HTML (
typeconversion.html,typecorecion.html,templateliterals.html), which goes against the best practices outlined in the PR description. Scripts should be in external files. - One instance of inline event handling (
onload) should be replaced with modern event listeners. - CSS is embedded in an HTML file instead of a separate stylesheet.
- CSS files use hardcoded colors instead of CSS variables, which harms maintainability.
- Several files mix JavaScript and HTML (
Please review the detailed comments for suggestions on how to address these issues. As a minor note, there is a typo in the filename javascript/2-vars-data-types/Assignment-5/typecorecion.html.
|
|
||
|
|
||
| <script> | ||
| document.getElementById("internal").innnerHTML = "Hello from Internal JS!"; |
There was a problem hiding this comment.
There's a typo in innnerHTML; it should be innerHTML. This error will prevent the JavaScript from updating the element's content, causing a bug in the page's functionality.
| document.getElementById("internal").innnerHTML = "Hello from Internal JS!"; | |
| document.getElementById("internal").innerHTML = "Hello from Internal JS!"; |
| <meta charset="UTF-8"> | ||
| <title>Inline, Internal & External JS Execution Order</title> | ||
| </head> | ||
| <body onload="document.getElementById('inline').innerHTML = 'Hello from Inline JS!'"> |
There was a problem hiding this comment.
Using the onload attribute for event handling is an outdated practice that mixes JavaScript logic with HTML markup. The PR description's best practices advise against this. A modern and cleaner approach is to use window.addEventListener('load', ...) within a script tag to separate concerns.
| try { | ||
| let y = 20; | ||
| output += "After redeclaration: " + y + "<br>"; | ||
| } catch (err) { | ||
| output += "Error on redeclaration: " + err.message + "<br>"; | ||
| } |
There was a problem hiding this comment.
This try...catch block does not correctly demonstrate an error for redeclaring a variable with let. The let y = 20; inside the try block is in a new scope and does not conflict with the y on line 11. A SyntaxError for redeclaration occurs at parse time and cannot be caught by try...catch. This makes the demonstration misleading. The same logical flaw applies to the const redeclaration example on lines 25-30. To show this behavior, you could use commented-out code with an explanation of the error it would produce.
| } | ||
|
|
||
| header { | ||
| background: #0d0a99; |
There was a problem hiding this comment.
The color #0d0a99 is hardcoded. For better maintainability and to ensure a consistent color scheme, it's recommended to define colors as CSS Custom Properties (variables) in a :root selector. This practice should be applied to all hardcoded colors in the file (e.g., on lines 41, 49, 56).
| <style> | ||
| body { | ||
| font-family: Arial, sans-serif; | ||
| margin: 0; | ||
| padding: 0; | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| height: 100vh; | ||
| background: #f0f0f0; | ||
| } | ||
| .box { | ||
| width: 150px; | ||
| height: 150px; | ||
| background: #3498db; | ||
| border-radius: 10px; | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| color: white; | ||
| font-weight: bold; | ||
| animation: bounce 2s infinite; | ||
| } | ||
| @keyframes bounce { | ||
| 0%, 100% { | ||
| transform: translateY(0); | ||
| } | ||
| 50% { | ||
| transform: translateY(-50px); | ||
| } | ||
| } | ||
| @media (max-width: 768px) { | ||
| .box { | ||
| width: 120px; | ||
| height: 120px; | ||
| font-size: 14px; | ||
| background: #e67e22; | ||
| } | ||
| } | ||
|
|
||
| @media (max-width: 480px) { | ||
| .box { | ||
| width: 90px; | ||
| height: 90px; | ||
| font-size: 12px; | ||
| background: #2ecc71; | ||
| } | ||
| } | ||
| </style> |
There was a problem hiding this comment.
The CSS styles are defined within a <style> tag in the HTML file. While this works for small examples, it goes against the best practice of separating concerns, which you've acknowledged in the PR description. Moving these styles to an external .css file improves modularity, reusability, and allows the browser to cache the stylesheet.
| <script> | ||
| let output = ""; | ||
|
|
||
| let str = "123"; | ||
| let num = 42; | ||
| let bool = true; | ||
| let undef; | ||
| let nul = null; | ||
| let sym = Symbol("id"); | ||
| let obj = {name:"Sai"}; | ||
|
|
||
| function convertAndPrint(value, typeName) { | ||
|
|
||
| let displayValue = (typeof value === "symbol") ? value.toString() : value; | ||
| output += `<b>${typeName} (${displayValue})</b><br>`; | ||
|
|
||
| try { | ||
| let n = Number(value); | ||
| output += "Number(): " + n + "<br>"; | ||
| } catch(e) { | ||
| output += "Number(): Error - " + e.message + "<br>"; | ||
| } | ||
|
|
||
| try { | ||
| let s = String(value); | ||
| output += "String(): " + s + "<br>"; | ||
| } catch(e) { | ||
| output += "String(): Error - " + e.message + "<br>"; | ||
| } | ||
|
|
||
| try { | ||
| let b = Boolean(value); | ||
| output += "Boolean(): " + b + "<br><br>"; | ||
| } catch(e) { | ||
| output += "Boolean(): Error - " + e.message + "<br><br>"; | ||
| } | ||
| } | ||
| convertAndPrint(str, "String"); | ||
| convertAndPrint(num, "Number"); | ||
| convertAndPrint(bool, "Boolean"); | ||
| convertAndPrint(undef, "Undefined"); | ||
| convertAndPrint(nul, "Null"); | ||
| convertAndPrint(sym, "Symbol"); | ||
| convertAndPrint(obj, "Object"); | ||
|
|
||
| document.getElementById("output").innerHTML = output; | ||
| </script> |
There was a problem hiding this comment.
This HTML file contains a substantial amount of JavaScript. According to the best practices in the PR description, scripts should be in separate files to improve organization, maintainability, and enable caching. Please move this script to an external .js file. This feedback also applies to Assignment-5/typecorecion.html and Assignment-6/templateliterals.html.
1 participant
Terms and Conditions
HTML Best Practices
File Naming Convention:
Follow consistent and descriptive naming (e.g., dashboard.html, user-profile.html).
Use lowercase letters and hyphens instead of spaces.
Page Title:
Ensure the <title> tag is descriptive and aligns with the page content.
Include meaningful keywords for SEO if applicable.
Semantic Markup:
Use appropriate tags like <header>, <footer>, <section>, <article> for better readability and accessibility.
Accessibility Standards:
Ensure the use of alt attributes for images and proper labels for form elements.
Use ARIA roles where necessary.
Validation:
Ensure the code passes HTML validation tools without errors or warnings.
Structure and Indentation:
Maintain consistent indentation and proper nesting of tags.
Attributes:
Ensure all required attributes (e.g., src, href, type, etc.) are correctly used and not left empty.
CSS Best Practices
File Organization:
Use modular CSS files if applicable (e.g., base.css, layout.css, theme.css).
Avoid inline styles unless absolutely necessary.
Naming Conventions:
Use meaningful class names following BEM or other conventions (e.g., block__element--modifier).
Code Reusability:
Avoid duplicate code; use classes or mixins for shared styles.
Responsive Design:
Ensure proper usage of media queries for mobile, tablet, and desktop views.
Performance Optimization:
Minimize the use of unnecessary CSS selectors.
Avoid overly specific selectors and ensure selectors are not overly deep (e.g., avoid #id .class1 .class2 p).
Consistency:
Follow consistent spacing, indentation, and use of units (rem/em vs. px).
Maintain a single coding style (e.g., always use double or single quotes consistently).
Javascript Best Practices
File Organization:
Ensure scripts are modular and logically separated into files if needed.
Avoid mixing inline JavaScript with HTML.
Logic Optimization:
Check for redundancy and ensure the code is optimized for performance.
Avoid unnecessary API calls or DOM manipulations.
Solution Approach:
Confirm that the code solves the given problem efficiently.
Consider scalability for future enhancements.
Readability:
Use clear variable and function names.
Add comments for complex logic or algorithms.
Error Handling:
Ensure proper error handling for API calls or user input validation.
Code Quality:
Check for potential bugs (e.g., missing await, mishandling of null/undefined values).
Avoid unnecessary console.log statements in production code.
Security:
Avoid hardcoding sensitive data.
Sanitize user input to prevent XSS and other vulnerabilities.
Best Practices:
Use const and let instead of var.
Follow ES6+ standards where applicable.